-
Notifications
You must be signed in to change notification settings - Fork 2
CMS-45730 Add alloy template #139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
892c021 to
0277cd3
Compare
…out and content handling; enhance Teaser component with link wrapping functionality.
0277cd3 to
1458a0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new alloy-template to the templates directory, providing a Next.js-based starter template for Optimizely CMS projects. The template includes pre-configured components, TypeScript configuration, and Tailwind CSS styling.
- Adds a complete Next.js 15.5.4 template with React 19.1.0
- Includes various reusable CMS components (Teaser, Notice, Contact, etc.)
- Configures TypeScript, Tailwind CSS v4, and PostCSS
- Updates workspace configuration to include the templates directory
Reviewed Changes
Copilot reviewed 28 out of 36 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/alloy-template/package.json | Defines project dependencies including Next.js, React 19, and Tailwind CSS v4 |
| templates/alloy-template/tsconfig.json | TypeScript configuration with Next.js and bundler module resolution |
| templates/alloy-template/src/components/*.tsx | Component files for Teaser, Notice, Contact, Editorial, Article, and other CMS content types |
| templates/alloy-template/src/components/Start.tsx | Start page component with composition support |
| templates/alloy-template/src/app/*.tsx | Next.js app router pages including layout, dynamic routes, and preview |
| templates/alloy-template/next.config.ts | Next.js configuration for remote image patterns |
| pnpm-workspace.yaml | Updated to include templates directory in workspace |
| pnpm-lock.yaml | Lock file updated with new template dependencies |
Files not reviewed (2)
- pnpm-lock.yaml: Language not supported
- templates/alloy-template/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const PageList = contentType({ | ||
| key: 'PageListBlock', | ||
| displayName: '', | ||
| description: '', |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The displayName and description fields are empty strings. These should be populated with meaningful values to improve the component's discoverability and documentation in the CMS. Consider setting displayName: 'Page List Block' and providing a helpful description.
| console.log(opti.image?.url.default); | ||
|
|
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug console.log statement should be removed before production. This appears to be leftover debugging code.
| console.log(opti.image?.url.default); |
| } | ||
|
|
||
| function StartPage({ opti }: StartPageProps) { | ||
| const { pa } = getPreviewUtils(opti); |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable pa.
| const { pa } = getPreviewUtils(opti); |
| @@ -0,0 +1,52 @@ | |||
| import { contentType } from '@optimizely/cms-sdk'; | |||
|
|
|||
| const PageList = contentType({ | |||
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable PageList.
| } from '@optimizely/cms-sdk/react/server'; | ||
|
|
||
| export const StandardContentType = contentType({ | ||
| key: 'Standard', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We seem to be a bit inconsistent with content type naming. Some content types have the suffix 'Page' and some not.
| key: 'Standard', | ||
| displayName: 'Standard Page', | ||
| baseType: '_experience', | ||
| mayContainTypes: ['_self', 'News'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it not possible to combine strings with objects, like mayContainTypes: ['_self', News]?
| type: 'string', | ||
| displayName: 'Page Description', | ||
| }, | ||
| disable_indexing: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SEO properties should probably be grouped into a block/component. Then use the block as a property on page types. Might make more sense to define them in a contract when we have that support, but until then a block property makes sense.
We have both title and site_title. Same with description.
| displayName: 'Button Link', | ||
| group: 'Information', | ||
| }, | ||
| button_text: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this button really needed? If yes, then define it as a block/compontent and use it as a block property rather than two separate properties here.
This template is 75% complete. The remaining features can be implemented once the tasks in CMS-46339 are resolved.